Fix possible image recursive loading #3897
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a potential infinite recursion issue in image loading where missing images could cause repeated loading attempts. The change replaces the missing image check with a loaded flag to prevent multiple loading operations.
- Introduces a boolean flag to track image loading state
- Replaces missing image comparison with loaded flag check to prevent recursive loading
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughIntroduced a one-time lazy initialization gate for asynchronous icon loading in PluginViewModel.Image using a new _imageLoaded flag, replacing the previous MissingImage-based trigger. LoadIconAsync behavior and public API remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI/Consumer
participant VM as PluginViewModel
participant Loader as LoadIconAsync
UI->>VM: Access Image
alt First access (_imageLoaded == false)
VM->>VM: _imageLoaded = true
VM->>Loader: LoadIconAsync()
activate Loader
Loader-->>VM: Set Image, OnPropertyChanged(Image)
deactivate Loader
else Subsequent access
VM-->>UI: Return cached Image
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (1)
40-41: Rename _imageLoaded to _imageLoadStarted for semantic clarityThe flag is set before loading completes and is used as a “started” gate, not a “loaded” indicator. Renaming reduces confusion and aligns field name with behavior.
Apply this diff:
- private bool _imageLoaded = false; + private bool _imageLoadStarted = false; @@ - if (!_imageLoaded) + if (!_imageLoadStarted) { - _imageLoaded = true; + _imageLoadStarted = true; _ = LoadIconAsync(); }Alternatively, if you truly want a “loaded” (completed) signal, set a separate flag at the end of LoadIconAsync and keep this one as “started.” This also gives you a place to decide whether/how to retry if the first attempt fails.
Also applies to: 46-51
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/ViewModel/PluginViewModel.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Flow.Launcher/ViewModel/PluginViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
…ading Fix possible image recursive loading
2 participants
Fix possible image recursive loading
If this image is missing,
_image == ImageLoader.MissingImagewill always be true, which can cause recursive loading. So we should use loaded flag to indicate this